/cc @davmason @noahfalk

@k15tfu do you intend for this PR to serve as an API review? There is no implementation for the method here, so I am not sure what you intend.

@davmason For the first time - yes. I wanted to be punctual and get the sign off first. After that I gonna update this PR and provide its implementation.

Anyway, I already have a draft for SetEnvironmentVariable: 6724736

0984ef0 - added interface for GetEnvironmentVariable

The signature for GetEnvironmentVariable looks good, I haven't looked at anything else yet

260734d - added impl for GetEnvironmentVariable

6d733da - make them async

bbbd1c4 - add szName check in SetEnvironmentVariable

298f430 - SetEnvironmentVariable returns GetLastError() in case of error

53c9792 - rebase against ToT.

@davmason now I think it's ready for review.

I think it looks good. What testing have you done? We should confirm that environment variables flow the way you expect, if you set one a child process inherits it, and you can pick it up from managed code, etc.

CC @janvorli in case he has anything to add.

f712d07 - fix log messages

0aa7e76 - GetEnvironmentVariable returns S_OK (not ERROR_INSUFFICIENT_BUFFER) if buffer is nullptr. For example, GetEnvironmentVariable(u"TEST_MY_VAR", 0, &outSize, nullptr) will always return S_OK if TEST_MY_VAR exists.

15ed902 - code style fixes

c4c0ede - rebase against ToT

@davmason Yes, everything is fine now. I can get, set a new, override, or delete an environment variable as we expect. Also, after setting a new variable in Initialize callback it's passed into child processes.

@davmason I have also checked that it can be used asynchronously: I did a test, where C# in a loop updates env_var := env_var + 1 if env_var is even, and the profiler does the same thing but if env_var is odd. After 5 seconds they reached 120K.

@davmason Nice. What should we do next to backport it to 3.1? Could you create a PR for that?

@k15tfu I am starting the process of requesting the 3.1 port. I should know more early next week.

One of the things I will be asked is what is the impact of not having this. If you could provide information about how this will impact your profiler that would help.

@davmason Without this, we can lost all of our features related to child processes: 1) ability to profile child processes (for details see https://github.com/dotnet/coreclr/issues/23383#issuecomment-514587669), 2) ability to take snapshots automatically depending on child processes hierarchy, 3) unit-testing profiler support. Most likely, end user experience will be significantly worsened if our "tricks" stop working.